Skip to content

fix(sqlite): escape SQL identifiers and literals in metadata/DDL paths (#1914) - #2204

Merged
openai0229 merged 3 commits into
OtterMind:mainfrom
HandSonic:fix/sqli2-sqlite
Jul 29, 2026
Merged

fix(sqlite): escape SQL identifiers and literals in metadata/DDL paths (#1914)#2204
openai0229 merged 3 commits into
OtterMind:mainfrom
HandSonic:fix/sqli2-sqlite

Conversation

@HandSonic

@HandSonic HandSonic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Related issue

Closes #1914

Summary

Completes the SQLite database-plugin portion of the identifier and literal escaping work.

  • Adds SQLite reserved-word handling and preserves the shared always-quote/remove round-trip contract for plain, quoted, embedded-delimiter, blank, and qualified-name inputs.
  • Uses unconditional identifier quoting across metadata, export, create/alter/index, inherited select/count/insert/update/drop, and DML template paths.
  • Replaces permissive type/default regular expressions with quote-aware balanced scanning that preserves legal nested SQLite expressions while rejecting comments, statement boundaries, top-level commas, unbalanced input, and appended column constraints.
  • Makes SQLite default-value support reachable for built-in affinity types and preserves legal functions such as strftime(...) and json_extract(...).
  • Escapes metadata lookup literals, flattens all supported line terminators in generated -- comments, and keeps null database qualifiers out of generated SQL.
  • Corrects SQLite SELECT template generation so it emits FROM rather than the malformed shared FROM where constant.

Affected surfaces

  • Frontend / Web
  • Backend / API / Storage
  • Database plugin / Driver
  • JCEF / Desktop packaging
  • CI / Build / Release
  • Documentation only

Verification

  • Commands and results:
    • mvn -B -f chat2db-community-server/pom.xml -pl chat2db-community-plugins/chat2db-community-sqlite -am clean test -DskipTests=false -Dmaven.test.skip=false -Dsurefire.failIfNoSpecifiedTests=false
    • Result: BUILD SUCCESS across 9 reactor modules; 791 tests passed in tested modules, including SQLite 22/22, MySQL 653/653, SPI 90/90, and tools 26/26.
    • git diff --check origin/main...HEAD: passed.
  • Manual verification: Executed representative quoted CREATE TABLE, INSERT, UPDATE, CREATE INDEX, and SELECT statements with /usr/bin/sqlite3 :memory:; the final query returned updated.
  • UI evidence: N/A

Risk and compatibility

  • Public API or stored data: No public API or stored-data schema change. Uses the existing shared ISQLIdentifierProcessor contract.
  • Database or driver compatibility: Conditional SPI quoting remains available for completion/matching; metadata-driven generated SQL always quotes raw identifiers. Legal SQLite affinity types, collations, literals, and nested default expressions are covered by regression tests and CLI execution.
  • Network, privacy, or security: No network or privacy change. Reduces malformed SQL and identifier/literal injection risk in generated metadata, DDL, export, and DML paths.
  • Community / Local / Pro boundary: Community SQLite plugin only; Local and Pro consume the Community plugin behavior without product-specific code changes.
  • Backward compatibility: Ordinary identifiers and values retain their meaning; reserved words, embedded quotes, custom type expressions, defaults, and unusual metadata names are now encoded or validated consistently.

Reviewer map

  • Start here: SqliteIdentifierProcessor, SqliteSqlGuards, then SqliteBuilder, SqliteMetaData, SqliteDBManager, and the type/index enums.
  • Failure condition: Quote/remove loses raw text, legal defaults/types are rewritten, generated SQL contains raw metadata identifiers/literals, null qualifiers are rendered, or DML templates are syntactically malformed.
  • Rollback or disable path: Revert this PR; no migration or persisted-data rollback is required.

Contributor declaration

  • I linked the Issue that defines this change.
  • I tested the affected behavior and reported the actual results above.
  • I did not include credentials, private data, or generated build output.
  • I disclosed substantial AI assistance below, or this PR contains no substantial AI-generated code.

AI assistance: Substantial AI-assisted maintainer revision, multi-agent review, and test expansion; all final changes were source-reviewed and verified with the commands above.

HandSonic and others added 3 commits July 29, 2026 17:59
…intainer review (OtterMind#1914)

- SqliteIdentifierProcessor gains INSTANCE, escapeString (single-quote
  doubling) and static escapeIdentifier; quoteIdentifier overloads delegate
- SqliteMetaData/DBManager use getSQLIdentifierProcessor()/INSTANCE
- builders/enums use SqliteIdentifierProcessor.escapeIdentifier
- non-escapable validation moved to SqliteSqlGuards (safe names, type
  names, column defaults, line comments)
- SqliteSqlEscapes removed; tests migrated (16 green)

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer verification completed against the shared identifier round-trip contract and SQLite generated-SQL paths. Clean 9-module reactor passed; SQLite tests 22/22. No blocking findings.

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer review complete. SQLite identifier/literal escaping, structural type/default guards, inherited builder paths, regression tests, and local sqlite3 execution were revalidated on the latest main.

@openai0229
openai0229 merged commit d276a0f into OtterMind:main Jul 29, 2026
22 checks passed
@openai0229 openai0229 moved this from In Review to Done in Chat2DB Community Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Security: 34 SQL Injection Vulnerabilities in Database Plugins

2 participants